Minting Handles For Datasets #3146

jo-pol · 2016-05-31T08:03:11Z

Implementation of Minting Handles For Datasets, see BRD

Some refactoring makes the dataset commands classes agnostic which IdServiceBean is configured: DOIEZIDServiceBean, DOIDataciteServiceBean or HandlenetServiceBean. When an id is registered is hard coded per bean: upon creation for EZID, upon publishing for DataCite and Handlenet.

1. Related Issues

Handles: Restore handle registration functionality in 4.x #2437 Handles: Restore handle registration functionality in 4.x
the ServiceBeans implementing an interface/abstract-class meets the Registration API halfway
see also the initial discussion

2. Pull Request Checklist

Functionality completed as described in FRD
Dependencies, risks, assumptions in FRD addressed
Unit tests completed
Deployment requirements identified (e.g., SQL scripts, indexing)
Documentation completed
All code checkins completed

3. Review Checklist

After the pull request has been submitted, fill out this section.

Code review completed or waived
Testing requirements completed
Usability testing completed or waived
Support testing completed or waived
Merged with develop branch and resolved conflicts

… warnings

coveralls · 2016-05-31T08:09:28Z

Coverage increased (+0.007%) to 5.138% when pulling 84eb5d4 on jo-pol:DDN-184-DOI-interface into 2935cde on IQSS:develop.

jo-pol · 2016-06-28T16:51:52Z

@4tikhonov
At last I found my work done so far.

@janvanmansum
I suppose we will need a fork at DANS-KNAW with a dans-develop-4.x branch (like IISH) and figure out a branching strategy to get things merged into the develop branch of IQSS

djbrooke · 2016-08-02T16:18:09Z

@scolapasta is going to correspond with @jo-pol about this one.

coveralls · 2016-08-04T13:46:53Z

Coverage increased (+0.009%) to 6.791% when pulling b1f171b on jo-pol:DDN-184-DOI-interface into 054cafb on IQSS:develop.

…w extends IdServiceBean, implemented as far as it used to be called

coveralls · 2016-08-05T13:38:58Z

Coverage increased (+0.006%) to 6.789% when pulling 3d87237 on jo-pol:DDN-184-DOI-interface into e31b7d5 on IQSS:develop.

jo-pol · 2016-08-16T06:59:54Z

In the process of testing.

Needed to base my own changes on 47b04ed which is tagged with 4.4, see new branch with additional refactoring. Only then http://ddvn.dans.knaw.nl:8080/dataverse.xhtml (our vagrant instance, a private variant of dataverse-ansible) did not throw an internal error by DataverseRoleServiceBean (column "privateurltoken" does not exist b629598) after /usr/local/glassfish4/bin/asadmin deploy of my own dataverse.war Figured out I needed to execute scripts/database/upgrades/upgrade_v4.4_to_v4.5.sql
Creating a dataset succeeds ~~but I haven't seen my fine logging yet in /usr/local/glassfish4/glassfish/domains/domain1/logs/server.log despite a asadmin set-log-levels of the classes that I changed.~~

update

deaccession also seems OK as it logs errors nor warnings, managed to republish once but don't know how to reproduce
trigger UpdateDatasetTargetURLCommand
replace the default configuration with EZID by Datacite
provoke/mock exceptions

jo-pol · 2016-08-19T09:26:10Z

getLogger is usually called with .class.getCanonicalName, I also see .class.getName and .class.getPackage().getName() but worst of all: hardcoded strings for DOIDataCiteServiceBean, DOIEZIdServiceBean (with a wrong package name), HandlenetServiceBean and DataciteMetadataTemplate.

Should that be fixed?

jo-pol · 2016-08-19T10:27:00Z

CreateDatasetCommand sets GlobalIdCreateTime if createIdentifier was successful for EZId. Subsequently UpdateDatasetCommand retries if GlobalIdCreateTime is missing. Why is this mechanism not implemented for DataCite? In both cases createIdentifier can fail.

pdurbin · 2016-08-19T17:13:15Z

I pinged @sekmiller about these questions and he'll be working with @jo-pol on answering them.

(cherry picked from commit c269368)

coveralls · 2016-08-22T07:04:32Z

Coverage decreased (-0.009%) to 6.773% when pulling c92614c on jo-pol:DDN-184-DOI-interface into e31b7d5 on IQSS:develop.

coveralls · 2016-08-23T08:50:43Z

Coverage decreased (-0.008%) to 6.775% when pulling a56a771 on jo-pol:DDN-184-DOI-interface into e31b7d5 on IQSS:develop.

# Conflicts: # src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DeaccessionDatasetVersionCommand.java

coveralls · 2016-08-25T13:51:43Z

Coverage decreased (-0.004%) to 7.927% when pulling cfd7f26 on jo-pol:DDN-184-DOI-interface into dc58ae1 on IQSS:develop.

sekmiller · 2016-08-29T16:33:24Z

Summarizing what I said in our chat previously:

The UpdateDatasetTargetURLCommand is triggered via the native API
with the following: GET http://$server/api/datasets/modifyRegistrationAll

It is only something that an admin would use since it updates the Target URL at the DOI service provider. We used it on the deployment of 4.0 and also when we added more widget functionality in 4.2 or 4.3 since these resulted in a change to Dataverse's base URL

sekmiller · 2016-08-29T16:37:38Z

With regard to the UpdatDatasetCommand not trying to register DataCite datasets. This is because DataCite does not allow for a reserved (non-public) identifier. On create of a DataCite dataset we get an identifier for the dataset, but we don't actually register it with DataCite until the dataset is published. It's at that time we will register it with DataCite.

sekmiller · 2016-08-29T16:43:24Z

We should definitely fix the logger calls. Thanks for picking up on that.

Also, with respect to mock/provoke exceptions: we do need to add more unit testing. There was some work done regarding the base testing of commands that I think was merged into develop recently. I will take a look at it and see if it will help with adding testing here.

#3146 (comment)

coveralls · 2016-08-30T09:13:57Z

Coverage decreased (-0.005%) to 7.926% when pulling 5c35dc4 on DANS-KNAW-jp:DDN-184-DOI-interface into dc58ae1 on IQSS:develop.

jo-pol · 2016-08-30T09:43:55Z

src/main/java/edu/harvard/iq/dataverse/engine/command/impl/CreateDatasetCommand.java

                } catch (Exception e){
                    logger.log(Level.WARNING, "Exception while creating Identifier: " + e.getMessage(), e);
                }

                // Check return value to make sure registration succeeded
-                if (doiProvider.equals("EZID") && doiRetString.contains(theDataset.getIdentifier())) {
+                if (!idServiceBean.registerWhenPublished() && doiRetString.contains(theDataset.getIdentifier())) {


It's my intention to drop the check on the protocol once implementation of the handle service bean is completed.

There is still DestroyDatasetCommand calling ctxt.doiDataCite(). The last DatasetCommand that is not agnostic about the configured bean by calling IdServiceBean.getBean(ctxt) controlled with another property of an idServiceBean. Don't yet know how that property should be named and what the value for both properties should be for handlenet. Even I'm not sure whether these properties should be hard-coded or configurable. I also remember Ben wanted a handle to point to some resource with some original meta data and additional provenance about why/when the dataset was destroyed and who requested/executed/approved the action.

Rediscovering the API I guess the interface should be renamed into IdRegistrationServiceBean.

Yes, please rename this using PersistentIdRegistrationServiceBean instead of IdRegistrationServiceBean.

For the AbstractIdServiceBean, please add a postDeleteCleanup cleanup method that will be called by the DestroyDatasetCommand.

public void postDeleteCleanup(){ ...}

Within the AbstractIdServiceBean, postDeleteCleanup can be added as a method that does nothing.

For the DataCite implementation, override the postDeleteCleanup method and add the DataCite cleanup

coveralls · 2016-08-30T11:15:13Z

Coverage decreased (-0.005%) to 7.926% when pulling 322ce23 on DANS-KNAW-jp:DDN-184-DOI-interface into dc58ae1 on IQSS:develop.

coveralls · 2016-09-01T08:37:07Z

Coverage decreased (-0.005%) to 7.926% when pulling cafe13b on DANS-KNAW-jp:DDN-184-DOI-interface into dc58ae1 on IQSS:develop.

jo-pol · 2016-09-02T07:51:11Z

src/main/java/edu/harvard/iq/dataverse/DOIDataCiteServiceBean.java

    public String getIdentifierForLookup(String protocol, String authority, String separator, String identifier) {
+        logger.log(Level.FINE,"getIdentifierForLookup");


This method seems the same to me for both EZId and Datacite and I suppose it should become the same for handlenet as well. If true we can pull this one up to the abstract service bean. Any future exception to the rule can still override.

coveralls · 2016-09-15T13:17:40Z

Coverage decreased (-0.004%) to 7.926% when pulling fd93ae8 on DANS-KNAW-jp:DDN-184-DOI-interface into dc58ae1 on IQSS:develop.

- pulled up methods to AbstractIdServiceBean - implemented methods of HandleNetServiceBean that still threw a NotImplementedException and were called somewhere This is a squashed commit of https://github.com/DANS-KNAW-jp/dataverse/pull/1 alias https://github.com/DANS-KNAW-jp/dataverse/tree/DDN-184-handle-implementation

coveralls · 2016-09-22T12:52:51Z

Coverage decreased (-0.004%) to 7.926% when pulling 354e624 on DANS-KNAW-jp:DDN-184-DOI-interface into 3a753ca on IQSS:develop.

raprasad

Please make the strings "doi" and "EZID" into re-usable variables.

pdurbin · 2016-09-30T16:36:53Z

src/main/java/edu/harvard/iq/dataverse/IdServiceBean.java

+
+    String generateYear();
+
+    String generateTimeString();


From what we can tell, the generateTimeString method isn't actually used anywhere. I commented it out as of 354e624 and the app still compiles. Should we remove it?

pdurbin · 2016-09-30T16:39:57Z

src/main/java/edu/harvard/iq/dataverse/IdServiceBean.java

+        String nonNullDefaultIfKeyNotFound = "";
+        String doiProvider = ctxt.settings().getValueForKey(SettingsServiceBean.Key.DoiProvider, nonNullDefaultIfKeyNotFound);
+
+        if ("hdl".equals(protocol))


Instead of hard coding "hdl" here can we make HANDLE_PROTOCOL_TAG public and reference that instead? That way we can define "hdl" just once.

raprasad · 2016-09-30T16:21:04Z

src/main/java/edu/harvard/iq/dataverse/DatasetServiceBean.java

@@ -236,7 +236,8 @@ public boolean isUniqueIdentifier(String userIdentifier, String protocol, String
        boolean u = em.createQuery(query).getResultList().size() == 0;
        String nonNullDefaultIfKeyNotFound = "";
        String doiProvider = settingsService.getValueForKey(SettingsServiceBean.Key.DoiProvider, nonNullDefaultIfKeyNotFound);
-        if (doiProvider.equals("EZID")) {
+        if (protocol.equals("doi") && doiProvider.equals("EZID")) {


Please make the strings "doi" and "EZID" into re-usable static variables.

raprasad · 2016-09-30T16:24:11Z

src/main/java/edu/harvard/iq/dataverse/AbstractIdServiceBean.java

+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+public abstract class AbstractIdServiceBean implements IdServiceBean {


Thanks for adding the AbstractIdServiceBean class! It's a great help!

raprasad · 2016-09-30T16:27:48Z

src/main/java/edu/harvard/iq/dataverse/AbstractIdServiceBean.java

+    }
+
+    @Override
+    public String generateYear()


generateYear this method is not needed. e.g. methods like getMetadataFromStudyForCreateIndicator will be overwritten

jo-pol · 2016-10-06T12:18:20Z

@4tikhonov : Sorry, no time to test these requested changes by myself

coveralls · 2016-10-06T12:19:45Z

Coverage decreased (-0.004%) to 8.592% when pulling a43e2e9 on DANS-KNAW-jp:DDN-184-DOI-interface into 301e34a on IQSS:develop.

kcondon · 2016-10-26T19:03:12Z

Hi @jo-pol,

I've just tested your handle code and found a few issues. I've updated the original ticket, #2437, with that info and tagged some of our developers who may be able to

Thanks for working on this!
Kevin

pdurbin · 2016-10-28T12:47:49Z

There's a related thread from this morning at https://groups.google.com/d/msg/dataverse-community/v6jgyewGqlI/ZYDnFfWqAgAJ

pdurbin · 2017-04-26T23:19:50Z

Replaced by pull request #3800. Closing.

implemented interface for DOI...ServiceBean, eliminated some compiler…

84eb5d4

… warnings

djbrooke assigned scolapasta Aug 2, 2016

Merge remote-tracking branch 'IQSS/develop' into DDN-184-DOI-interface

b1f171b

jo-pol added 2 commits August 5, 2016 15:31

implemented DOIEZIdServiceBean.alreadyExists; HandlenetServiceBean no…

c4c246f

…w extends IdServiceBean, implemented as far as it used to be called

Merge remote-tracking branch 'IQSS/develop' into DDN-184-DOI-interface

3d87237

additional fine logging, static methods back to normal methods

0ba96ca

pdurbin assigned sekmiller and unassigned scolapasta Aug 19, 2016

jo-pol added 2 commits August 22, 2016 08:26

refactored DeaccessionDatasetVersionCommand too + fine logging

edd75c8

(cherry picked from commit c269368)

logging

c92614c

reusing code

a56a771

Merge remote-tracking branch 'IQSS/develop' into HEAD

cfd7f26

# Conflicts: # src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DeaccessionDatasetVersionCommand.java

decide by bean property, not by its name

5c35dc4

#3146 (comment)

jo-pol reviewed Aug 30, 2016
View reviewed changes

fixed logger constants for service beans

322ce23

jo-pol added 2 commits September 1, 2016 09:33

"contact dataverse support" appeared twice on the web page

1558680

restored control flow of former error handling

cafe13b

jo-pol reviewed Sep 2, 2016
View reviewed changes

jo-pol changed the title ~~Plugable id service beans: DOI(EZId/Datacite) handles~~ Minting Handles For Datasets Sep 16, 2016

pdurbin mentioned this pull request Sep 22, 2016

Handles: Restore handle registration functionality in 4.x #2437

Closed

raprasad reviewed Sep 30, 2016

View reviewed changes

pdurbin reviewed Sep 30, 2016

View reviewed changes

raprasad reviewed Sep 30, 2016

View reviewed changes

jo-pol added 3 commits October 6, 2016 09:23

Merge remote-tracking branch 'IQSS/develop' into DDN-184-DOI-interface

592b18b

rename to PersistentIdRegistrationServiceBean

ee0e6e0

strings to enums, generate-year/timeString, postDeleteCleanup

a43e2e9

djbrooke unassigned sekmiller Nov 30, 2016

pdurbin added the Feature: DOI & Handle label Feb 3, 2017

pdurbin closed this Apr 26, 2017

		public String getIdentifierForLookup(String protocol, String authority, String separator, String identifier) {
		logger.log(Level.FINE,"getIdentifierForLookup");

Minting Handles For Datasets #3146

Minting Handles For Datasets #3146

Conversation

jo-pol commented May 31, 2016 • edited by pdurbin

1. Related Issues

2. Pull Request Checklist

3. Review Checklist

coveralls commented May 31, 2016

jo-pol commented Jun 28, 2016

djbrooke commented Aug 2, 2016

coveralls commented Aug 4, 2016

coveralls commented Aug 5, 2016

jo-pol commented Aug 16, 2016 • edited

jo-pol commented Aug 19, 2016 • edited

jo-pol commented Aug 19, 2016

pdurbin commented Aug 19, 2016

coveralls commented Aug 22, 2016

coveralls commented Aug 23, 2016

coveralls commented Aug 25, 2016

sekmiller commented Aug 29, 2016

sekmiller commented Aug 29, 2016

sekmiller commented Aug 29, 2016

coveralls commented Aug 30, 2016

jo-pol Aug 30, 2016

Choose a reason for hiding this comment

raprasad Sep 30, 2016 • edited

Choose a reason for hiding this comment

raprasad Sep 30, 2016

Choose a reason for hiding this comment

coveralls commented Aug 30, 2016

coveralls commented Sep 1, 2016

jo-pol Sep 2, 2016

Choose a reason for hiding this comment

coveralls commented Sep 15, 2016

coveralls commented Sep 22, 2016

raprasad left a comment

Choose a reason for hiding this comment

pdurbin Sep 30, 2016

Choose a reason for hiding this comment

pdurbin Sep 30, 2016 • edited

Choose a reason for hiding this comment

raprasad Sep 30, 2016

Choose a reason for hiding this comment

raprasad Sep 30, 2016

Choose a reason for hiding this comment

raprasad Sep 30, 2016

Choose a reason for hiding this comment

jo-pol commented Oct 6, 2016

coveralls commented Oct 6, 2016

kcondon commented Oct 26, 2016

pdurbin commented Oct 28, 2016

pdurbin commented Apr 26, 2017

jo-pol commented May 31, 2016 •

edited by pdurbin

jo-pol commented Aug 16, 2016 •

edited

jo-pol commented Aug 19, 2016 •

edited

raprasad Sep 30, 2016 •

edited

pdurbin Sep 30, 2016 •

edited